Skip to content

Port lint attributes to attribute parser#152369

Open
Bryntet wants to merge 10 commits intorust-lang:mainfrom
Bryntet:lint_attrs
Open

Port lint attributes to attribute parser#152369
Bryntet wants to merge 10 commits intorust-lang:mainfrom
Bryntet:lint_attrs

Conversation

@Bryntet
Copy link
Copy Markdown
Contributor

@Bryntet Bryntet commented Feb 9, 2026

View all comments

Tracking issue: #131229

Ports #[allow], #[deny], #[expect], #[forbid], and #[warn] to being parsed attrs

I tried my best to make this PR as small as possible, it was difficult. I hope it isn't too difficult to review

r? @JonathanBrouwer
r? @jdonszelmann

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 9, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Feb 22, 2026
@Bryntet Bryntet changed the title lint attrs Port lint attributes to attribute parser Feb 22, 2026
@Bryntet Bryntet marked this pull request as ready for review February 22, 2026 18:57
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 22, 2026

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 22, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Feb 22, 2026
Port lint attributes to attribute parser
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 22, 2026
@Bryntet Bryntet force-pushed the lint_attrs branch 2 times, most recently from e70b414 to d9434c9 Compare February 22, 2026 20:19
@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 24, 2026
@rust-log-analyzer

This comment has been minimized.

@Bryntet
Copy link
Copy Markdown
Contributor Author

Bryntet commented Mar 25, 2026

@bors try

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Mar 25, 2026
Port lint attributes to attribute parser
@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 25, 2026
@Bryntet
Copy link
Copy Markdown
Contributor Author

Bryntet commented Mar 25, 2026

@rustbot ready

cc @jdonszelmann
since you asked for a ping when I had fixed the remaining comments from jonathan

another perf run is queued, because I made a change that hopefully has positive perf compared to the previous (made it so we consolidate all lint attrs into one LintAttributeKind, and then we sort it in finalize instead of having to allocate a new vector, sort it, and then sort that multiple times)

@rust-log-analyzer

This comment has been minimized.

@Bryntet
Copy link
Copy Markdown
Contributor Author

Bryntet commented Mar 25, 2026

apparently running ./x test wasn't enough to catch all issues, sorry.

@rustbot author

Bryntet added 2 commits March 25, 2026 15:12
also add attr_id to `Stable` variant directly, instead of having to
iterate over all the attrs on the hir_id to find it
…tr parser

also changes method `parse_limited_all` to take Iterator as an input,
to avoid needing to do expensive allocation
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 25, 2026

☀️ Try build successful (CI)
Build commit: 196eb6d (196eb6dc5b065987b5111bc690fb1d337ac87395, parent: 8a703520e80d87d4423c01f9d4fbc9e5f6533a02)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (196eb6d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.1% [0.1%, 0.1%] 1
Regressions ❌
(secondary)
0.2% [0.2%, 0.3%] 3
Improvements ✅
(primary)
-0.3% [-0.5%, -0.2%] 16
Improvements ✅
(secondary)
-0.3% [-0.5%, -0.0%] 31
All ❌✅ (primary) -0.3% [-0.5%, 0.1%] 17

Max RSS (memory usage)

Results (primary -1.2%, secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
6.2% [3.9%, 9.5%] 5
Improvements ✅
(primary)
-1.5% [-2.4%, -0.9%] 5
Improvements ✅
(secondary)
-3.0% [-4.6%, -0.7%] 3
All ❌✅ (primary) -1.2% [-2.4%, 0.6%] 6

Cycles

Results (primary 3.4%, secondary 4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.4% [2.4%, 4.1%] 6
Regressions ❌
(secondary)
4.2% [2.5%, 5.7%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.4% [2.4%, 4.1%] 6

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 482.838s -> 485.094s (0.47%)
Artifact size: 394.91 MiB -> 396.87 MiB (0.50%)

@rust-log-analyzer

This comment has been minimized.

Bryntet added 4 commits March 29, 2026 16:06
also removes E0452 and splits
`tests/rustdoc-ui/lints/renamed-lint-still-applies` into 2 tests

this is because of delayed warn lint being lost on compiler aborting on
error
also buffer any lints coming from pre-expansion, so that they are
emitted with proper lint level
Comment on lines -183 to 197
[attr] => {
if matches!(Level::from_attr(attr), Some((Level::Expect, _)))
&& let metas = attr.meta_item_list()
&& let Some(lst) = metas
&& let [MetaItemInner::MetaItem(meta_item), ..] = lst.as_slice()
&& let [tool, lint_name] = meta_item.path.segments.as_slice()
&& tool.ident.name == sym::clippy
&& matches!(
lint_name.ident.name,
sym::needless_return | sym::style | sym::all | sym::warnings
)
[Attribute::Parsed(AttributeKind::LintAttributes(sub_attrs))] => {
if !sub_attrs
.into_iter()
.filter(|attr| attr.kind == LintAttributeKind::Expect)
.flat_map(|attr| &attr.lint_instances)
.any(|lint| {
matches!(
lint.original_name_without_tool(),
sym::needless_return | sym::style | sym::all | sym::warnings
)
})
{
// This is an expectation of the `needless_return` lint
} else {
return;
}
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: this does change behaviour of clippy, I think this behaviour change is a bug fix though.

Basically, previously it was just checking the first meta item, now it actually iterates over all expect attrs

Also we can't use find_attr in clippy for some reason, it emits an FCW (#120192)

Comment on lines -240 to +260
[attr]
if matches!(Level::from_attr(attr), Some((Level::Expect, _)))
&& let Some(metas) = attr.meta_item_list()
&& let Some(MetaItemInner::MetaItem(meta_item)) = metas.first()
&& let [tool, lint_name] = meta_item.path.segments.as_slice()
&& tool.ident.name == sym::clippy
&& [expected_lint_name, sym::style, sym::all].contains(&lint_name.ident.name) =>
{
// There is an `expect` attribute -- check that there is no _other_ significant text
let span_before_attr = inner_if.span.split_at(1).1.until(attr.span());
let span_after_attr = attr.span().between(inner_if_expr.span);
!span_contains_non_whitespace(cx, span_before_attr, self.lint_commented_code)
&& !span_contains_non_whitespace(cx, span_after_attr, self.lint_commented_code)
[
Attribute::Parsed(AttributeKind::LintAttributes(sub_attrs)),
] => {
sub_attrs
.into_iter()
.filter(|attr|attr.kind == LintAttributeKind::Expect)
.flat_map(|attr| attr.lint_instances.iter().map(|group| (attr.attr_span, group)))
.filter(|(_, lint_id)| {
lint_id.tool_is_named(sym::clippy)
&& (expected_lint_name == lint_id.lint_name()
|| [expected_lint_name, sym::style, sym::all]
.contains(&lint_id.original_name_without_tool()))
})
.any(|(attr_span, _)| {
// There is an `expect` attribute -- check that there is no _other_ significant text
let span_before_attr = inner_if.span.split_at(1).1.until(attr_span);
let span_after_attr = attr_span.between(inner_if_expr.span);
!span_contains_non_whitespace(cx, span_before_attr, self.lint_commented_code)
&& !span_contains_non_whitespace(cx, span_after_attr, self.lint_commented_code)
})
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see this comment

@Bryntet
Copy link
Copy Markdown
Contributor Author

Bryntet commented Mar 29, 2026

@rustbot ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants